-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
module: CJS / ESM resolver sharing and refactoring #34744
Conversation
Review requested:
|
Wait, so now you can map your exports to things in node_modules?? Can you help me understand why that's a good idea? |
Sure, I can add it back, it's just that the spec was actually incorrect since it was only checking the package.json target path, when we in fact need to check both the target path and the user subpath. We give separate errors for these two cases because the one is package author issue while the other is a package consumer issue. This error separation doesn't apply as easily for patterns though so those errors may need to be rethought. I refactored it out because it seems overly strict and unnecessary but I can add it back. |
I think it's very critical that "exports" only be possible to use to remap an entry specifier to "a file within the package that has exports". |
@ljharb I've added back the node_modules restriction identically to previously with a refactoring to make it part of the new segment check approach. |
734865c
to
fc1f44b
Compare
I would like to land this PR soon. Any further feedback welcome. |
@nodejs/modules-active-members for any further review |
PR-URL: #34744 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in f8976a7. |
Broken locally for me since f8976a7. Refs: nodejs#34744
PR-URL: #34744 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #34744 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#34744 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #34744 Backport-PR-URL: #35385 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: James M Snell <[email protected]>
This extracts just the resolver refactoring component from #34718.
I hope that the negative diff speaks for itself at least!
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes